Skip to content

Comments

[digiplex] Handle erroneous responses and restart the bridge#18035

Merged
jlaur merged 2 commits intoopenhab:mainfrom
rmichalak:17828-improve-error-handling
Jan 8, 2025
Merged

[digiplex] Handle erroneous responses and restart the bridge#18035
jlaur merged 2 commits intoopenhab:mainfrom
rmichalak:17828-improve-error-handling

Conversation

@rmichalak
Copy link
Contributor

@rmichalak rmichalak commented Jan 4, 2025 •
edited
Loading

Better handling of erroneous messages that may be received from the alarm system.

Based on the behavior of my alarm system, in case of an erroneous message there's only one way to restore proper communication: restart the bridge connection.

Supersedes: #17829 #17864

Closes #17828

@rmichalak rmichalak requested a review from jlaur January 4, 2025 16:49
@rmichalak
Copy link
Contributor Author

rmichalak commented Jan 4, 2025

@jlaur This is a compilation of my and your changes (including your test file).
I backported these changes to my old version of OH (2.5) and I will let it work for a week to make sure it's stable.

@rmichalak rmichalak marked this pull request as draft January 4, 2025 16:52
@rmichalak rmichalak changed the title Handle erroneous responses and restart the bridge [digiplex] Handle erroneous responses and restart the bridge Jan 4, 2025
@rmichalak rmichalak force-pushed the 17828-improve-error-handling branch from 988fc3b to cb228c4 Compare January 4, 2025 16:54
@rmichalak
[digiplex] Handle erroneous responses and restart the bridge
1853520 
Signed-off-by: Robert Michalak <rbrt.michalak@gmail.com>
@rmichalak rmichalak force-pushed the 17828-improve-error-handling branch from cb228c4 to 1853520 Compare January 4, 2025 16:56
jlaur
jlaur requested changes Jan 4, 2025
View reviewed changes
Copy link
Contributor

@jlaur jlaur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking care of this! Otherwise LGTM. Since most of the code origins from #17864 it would be best to have someone else review.

@rmichalak
[digiplex] fix wildcard imports
76165e8 
Signed-off-by: Robert Michalak <rbrt.michalak@gmail.com>
@rmichalak rmichalak force-pushed the 17828-improve-error-handling branch from e275b75 to 76165e8 Compare January 5, 2025 10:13
@jlaur jlaur added the bug An unexpected problem or unintended behavior of an add-on label Jan 5, 2025
@jlaur jlaur requested a review from a team January 5, 2025 11:33
@lsiepel
Copy link
Contributor

lsiepel commented Jan 5, 2025

@rmichalak this is marked as draft, is it ready to review and merge or are you waiting for something else?

@jlaur
Copy link
Contributor

jlaur commented Jan 5, 2025

is it ready to review and merge or are you waiting for something else?

See #18035 (comment). So it should be ready for review, but is still under test.

lsiepel
lsiepel approved these changes Jan 5, 2025
View reviewed changes
Copy link
Contributor

@lsiepel lsiepel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM.

Let's wait for test confirmation.

@rmichalak
Copy link
Contributor Author

rmichalak commented Jan 7, 2025

Okay, so I've just encountered a first malformed response: G01N008A01 - each number should have 3 digits but in this case two of them only have two and thus the whole message is shorter than expected... I may be smarter in parsing these things, but anyway... it's not a message that should appear on the line, according to Paradox documentation.

The good news is that the fix works (restarts the connection as expected), so we can merge it.

I will keep checking for these malformed messages as perhaps it's not necessary to restart the connection in each case. But that's a separate task.

@rmichalak rmichalak marked this pull request as ready for review January 7, 2025 17:42
@jlaur jlaur merged commit adacdeb into openhab:main Jan 8, 2025
2 checks passed
@jlaur jlaur added this to the 5.0 milestone Jan 8, 2025
This was referenced Jan 8, 2025
Closed
Closed
Merged
GearrelW pushed a commit to GearrelW/openhab-addons that referenced this pull request Jan 12, 2025
@rmichalak
[digiplex] Handle erroneous responses and restart the bridge (openhab…
9880a10 
…#18035)

Signed-off-by: Robert Michalak <rbrt.michalak@gmail.com>
chilobo pushed a commit to chilobo/openhab-addons that referenced this pull request Feb 10, 2025
@rmichalak @chilobo
[digiplex] Handle erroneous responses and restart the bridge (openhab…
41075c0 
…#18035)

Signed-off-by: Robert Michalak <rbrt.michalak@gmail.com>
Signed-off-by: Christian Koch <78686276+chilobo@users.noreply.github.com>
phenix1990 pushed a commit to phenix1990/openhab-addons that referenced this pull request Jul 31, 2025
@rmichalak @phenix1990
[digiplex] Handle erroneous responses and restart the bridge (openhab…
54b6a4f 
…#18035)

Signed-off-by: Robert Michalak <rbrt.michalak@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lsiepel lsiepel lsiepel approved these changes

@jlaur jlaur jlaur approved these changes

Assignees

No one assigned

Labels

bug An unexpected problem or unintended behavior of an add-on

Projects

None yet

Milestone

5.0

Development

Successfully merging this pull request may close these issues.

[digiplex] Received thread stops when an exception happens inside

3 participants

@rmichalak @lsiepel @jlaur